Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: filter out atlantis/apply from mergeability clause #1856

Merged
merged 7 commits into from
Oct 19, 2021

Conversation

nishkrishnan
Copy link
Contributor

@nishkrishnan nishkrishnan commented Oct 14, 2021

Just cherry picked the relevant commits from the lyft/atlantis. I'm going to remove the lyft specific things that aren't relevant and parameterize the status check name based on the server flag before this is ready for review.

Basically, this adds onto the mergeability checker in github client by fetching all the commit statuses and filtering out atlantis/apply from the status (in the case that mergeability returns false).

@nishkrishnan nishkrishnan marked this pull request as ready for review October 19, 2021 15:28
@nishkrishnan nishkrishnan requested a review from a team as a code owner October 19, 2021 15:28
@nishkrishnan nishkrishnan changed the title WIP Filter out atlantis/apply from mergeability clause Filter out atlantis/apply from mergeability clause Oct 19, 2021
@nishkrishnan
Copy link
Contributor Author

Should fix: #523

@@ -75,12 +74,15 @@ func (d *DefaultCommitStatusUpdater) UpdateCombinedCount(repo models.Repo, pull
return d.Client.UpdateStatus(repo, pull, status, src, fmt.Sprintf("%d/%d projects %s successfully.", numSuccess, numTotal, cmdVerb), "")
}

func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus, url string) error {
func (d *DefaultCommitStatusUpdater) UpdateProject(ctx models.ProjectCommandContext, command models.CommandName, status models.CommitStatus, url string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, cmdName might be better than command

Copy link
Member

@chenrui333 chenrui333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, pretty straightforward.

@chenrui333 chenrui333 changed the title Filter out atlantis/apply from mergeability clause feat: filter out atlantis/apply from mergeability clause Oct 19, 2021
Copy link
Contributor

@msarvar msarvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@nishkrishnan nishkrishnan merged commit d01796b into runatlantis:master Oct 19, 2021
@nishkrishnan nishkrishnan deleted the nish/filter-atlantis-apply branch October 19, 2021 20:51
nishkrishnan referenced this pull request in lyft/atlantis Nov 18, 2021
Filter out atlantis apply status during mergeability check.
nishkrishnan referenced this pull request in lyft/atlantis Nov 18, 2021
Filter out atlantis apply status during mergeability check.
nishkrishnan added a commit that referenced this pull request Jan 1, 2022
#1968)

* Revert "feat: filter out atlantis/apply from mergeability clause (#1856)"

This reverts commit d01796b.

* Add missing import.
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
…#1856)

Filter out atlantis apply status during mergeability check.
krrrr38 pushed a commit to krrrr38/atlantis that referenced this pull request Dec 16, 2022
runatlantis#1968)

* Revert "feat: filter out atlantis/apply from mergeability clause (runatlantis#1856)"

This reverts commit d01796b.

* Add missing import.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants